Skip to content

LCORE-740: check type hints for unit tests on CI#758

Merged
tisnik merged 1 commit intolightspeed-core:mainfrom
tisnik:lcore-727-check-type-hints-for-unit-tests-on-ci
Nov 4, 2025
Merged

LCORE-740: check type hints for unit tests on CI#758
tisnik merged 1 commit intolightspeed-core:mainfrom
tisnik:lcore-727-check-type-hints-for-unit-tests-on-ci

Conversation

@tisnik
Copy link
Contributor

@tisnik tisnik commented Nov 4, 2025 •
edited by coderabbitai bot
Loading

Description

LCORE-740: check type hints for unit tests on CI

Type of change

  • Refactor
  • New feature
  • Bug fix
  • CVE fix
  • Optimization
  • Documentation Update
  • Configuration Update
  • Bump-up service version
  • Bump-up dependent library
  • Bump-up library or tool used for development (does not change the final image)
  • CI configuration change
  • Konflux configuration change
  • Unit tests improvement
  • Integration tests improvement
  • End to end tests improvement

Related Tickets & Documents

  • Related Issue #LCORE-740

Summary by CodeRabbit

  • Chores
    • Updated type-checking configuration to validate both source and test code.
    • Adjusted type-checking strictness settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 4, 2025 •
edited
Loading

Walkthrough

The Makefile's check-types target was modified to expand mypy type-checking from src/ to both src/ and tests/unit/. The --disallow-incomplete-defs flag was removed from the mypy configuration.

Changes

Cohort / File(s) Summary
Build configuration
Makefile		 Updated check-types target to run mypy on src/ and tests/unit/ (was src/ only); removed --disallow-incomplete-defs flag from mypy options

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Possibly related PRs

Poem

🐰 Mypy now hops through the testing ground,
Strict rules relaxed, yet types abound,
From source to tests, the types align,
A rabbit's quest for code that's fine! ✨

Pre-merge checks and finishing touches

✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: extending mypy type checking from src/ to include unit tests in CI, which aligns with the Makefile modifications.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between dbe7714 and 6d117b9.

📒 Files selected for processing (1)
  • Makefile (1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings 
Learnt from: CR
Repo: lightspeed-core/lightspeed-stack PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-18T16:46:33.353Z
Learning: Applies to src/{models/**/*.py,configuration.py} : Use precise type hints in configuration (e.g., Optional[FilePath], PositiveInt, SecretStr)
📚 Learning: 2025-09-18T16:46:33.353Z 
Learnt from: CR
Repo: lightspeed-core/lightspeed-stack PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-18T16:46:33.353Z
Learning: Applies to tests/e2e/features/**/*.feature : Write E2E tests as Gherkin feature files for behave

Applied to files:

  • Makefile
📚 Learning: 2025-09-18T16:46:33.353Z 
Learnt from: CR
Repo: lightspeed-core/lightspeed-stack PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-18T16:46:33.353Z
Learning: Applies to tests/e2e/test_list.txt : Maintain the E2E test list in tests/e2e/test_list.txt

Applied to files:

  • Makefile
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
  • GitHub Check: build-pr
  • GitHub Check: Konflux kflux-prd-rh02 / lightspeed-stack-on-pull-request
  • GitHub Check: e2e_tests (ci)
  • GitHub Check: e2e_tests (azure)
🔇 Additional comments (2)
Makefile (2)

26-27: Inconsistency in summary: --disallow-incomplete-defs flag is still present.

The AI summary claims the --disallow-incomplete-defs flag was removed, but line 27 still contains this flag along with other strict type-checking options.

27-27: Verify that tests/unit/ passes strict type-checking or reconsider flag strictness for tests.

The check-types target now applies strict mypy flags—including --disallow-untyped-defs and --disallow-incomplete-defs—to unit tests. Test files often use pytest fixtures, mocks, and dynamic patterns that are difficult to type-hint fully. This could cause CI to fail if tests don't already have comprehensive type coverage.

Consider:

  1. Running mypy on tests/unit/ with these flags locally to verify compliance.
  2. If tests fail type-checking, decide whether to refactor tests to comply or use a less strict flag set for test files (e.g., excluding --disallow-incomplete-defs or using --ignore-missing-imports more liberally for test fixtures).

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@tisnik tisnik merged commit fa2121a into lightspeed-core:main Nov 4, 2025
18 of 22 checks passed
This was referenced Nov 10, 2025
Merged
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@tisnik